Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add source map tests #78

Merged
merged 3 commits into from
Dec 7, 2015

Conversation

novemberborn
Copy link
Contributor

Tests _rewritePath() as requested by @bcoe in #66.

I'd like to add a test for reading a source map file but I'm not quite sure where to put it. I mean I can test the add() method I suppose but there's no real test even verifying it's called. @bcoe any ideas? Happy with the coverage we have now?


This bumps source-map-fixtures. I've added a sourceFile path which can be used to create the mappedPath from #76.

I realized the generated coverage report already applied source maps. It now writes the raw coverage file instead.

I've added a few more fixtures (with coverage) to test _rewritePath().

The fixtures now provide a sourceFile property which can be used to get the
source location.
It can't be used to test source map support if the source maps are already
applied to the coverage report.

With this change no reports are written to disk. Therefore the temp directory
doesn't have to be cleaned.
@bcoe
Copy link
Member

bcoe commented Dec 7, 2015

@novemberborn one other annoying thing worth keeping in mind, I started doing a bunch of Windows testing on the weekend and, with the source-map fixtures, we run into issues with path.sep -- whether / or \ is used is all over the place:

  • istanbul seems to sometimes set ./.
  • we sometimes set ./.

Now, our core functionality does almost work on Windows without path.sep problems -- I hunted down the problem with spawning -- but the test suite is a ways of working; it might be smart to abstract the fixture lookup to not care about path.sep ... or something like that.

@novemberborn
Copy link
Contributor Author

it might be smart to abstract the fixture lookup to not care about path.sep ... or something like that.

source-map-fixtures uses path.join() so I think it's just the test itself which prefixes with ./.

Generate coverage reports for files without source maps or with multiple
sources.

Refactor source map tests a bit now that three covered fixtures are loaded.
@bcoe
Copy link
Member

bcoe commented Dec 7, 2015

@novemberborn the generated coverage file varies in path.sep usage too I think.

@bcoe
Copy link
Member

bcoe commented Dec 7, 2015

If you ever get adventurous, this is what I use for Windows Node testing, it's free and works pretty well on OSX:

https://github.com/xdissent/ievms

@novemberborn
Copy link
Contributor Author

@novemberborn the generated coverage file varies in path.sep usage too I think.

Ah yes it would. Maybe you could track progress in an issue and we can hash stuff out there? Can we use Travis to test Node on Windows?

@novemberborn
Copy link
Contributor Author

This is passing CI now.

@bcoe
Copy link
Member

bcoe commented Dec 7, 2015

@novemberborn AppVeyor seems to be the best available:

https://ci.appveyor.com/project/bcoe/yargs

Let's move Windows chatting to a separate ticket, there's enough problems with Windows right now that it shouldn't block this branch 👍

@bcoe
Copy link
Member

bcoe commented Dec 7, 2015

:shipit:

novemberborn added a commit that referenced this pull request Dec 7, 2015
@novemberborn novemberborn merged commit 08bb1c9 into istanbuljs:master Dec 7, 2015
@novemberborn novemberborn deleted the source-map-tests branch December 7, 2015 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants